Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed set- and clearTimeout to make debugging working with Firefox #9800

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixed set- and clearTimeout to make debugging working with Firefox #9800

wants to merge 1 commit into from
+2 −2

Conversation

keinhaar
Copy link

This fixes a problem that only occurs in firefox while debugging.
Without the fix SDBG Users would be forced to super-source the Impl class.
After a few weeks of testing i did'nt find any negative effects on runtime, or in other browsers.
Has been discussed in #9787.
Fixes #9787

@keinhaar keinhaar changed the title Fixed set- and clearTimeout to work in Firefox when debugging Fixed set- and clearTimeout to make debugging working with Firefox Feb 10, 2023
zbynek
zbynek suggested changes Mar 20, 2023
View reviewed changes
Copy link
Contributor

@zbynek zbynek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR is based on the patch that @niloc132 provided in the issue as a workaround with a comment

I don't think this is a reasonable fix to make to GWT itself, but it does look like we could remove the watchdog entirely, or only conditionally enable it.

so I think it would be better implement that instead.

@keinhaar
Copy link
Author

so I think it would be better implement that instead.
Maybe, but nobody is working on removing the watchdog, and this is a very simple fix.
In all of my projects it works without any problems.
So i propose to do this now, and later on, somebody could remove the watchdog.

@niloc132
Copy link
Member

Sorry for the delay on this. I finally simplified the bug report and filed it at https://bugzilla.mozilla.org/show_bug.cgi?id=1833637.

The watchdog timer isn't needed at all to see invalid behavior. Unfortunately, this shows that this patch doesn't go far enough - not only can the watchdog timer keep running, but any other timer can too - we have to fix any other call to $wnd.setTimeout to instead reference the local window/self, or we might hit other issues with this while a debugger is open. With the exception of RichTextAreaImplStandard and ScrollImpl, every call I see in GWT to setTimeout goes through $wnd, so will have this problem. Elemental2's DomGlobal.setTimeout will have this issue as well.

@keinhaar
Copy link
Author

keinhaar commented Sep 8, 2023

The same problem occurs in automated tests using selenium with the latest htmlunit-driver.
The proposed fix does resolve this too.

@niloc132
Copy link
Member

niloc132 commented Sep 8, 2023

That's interesting - did you file an issue with them showing the issue?

From the linked issue above, the underlying issue originally intended to be fixed by this patch is a regression in the FF debugger, with a workaround: In about:config, change devtools.every-frame-target.enabled to false. Note that changing GWT as this patch only fixes the bug for that specific timeout, but any other application code can still fail in this way, the best fix is to apply their workaround, since otherwise the bug can just come back another way.

@keinhaar
Copy link
Author

keinhaar commented Sep 8, 2023

I use the workaround in SDBG to avoid this problem.

@keinhaar keinhaar mentioned this pull request Sep 8, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zbynek zbynek zbynek requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to debug GWT in firefox
3 participants
@keinhaar @niloc132 @zbynek